-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Juliav0.7 fixes #61
Juliav0.7 fixes #61
Conversation
Ah thanks, I missed that one before tagging the new version. Maybe it would be better to use |
You're right, done. I also edited so 0.6, 0.7, and nightly are all tested and only nightly allows failures. |
Realized I goofed when I changed to Compat... my eyes are glazing over. Should be fixed shortly. Compat.norm still throws depwarns in v0.7, I wrapped the using in a version check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, added some comments.
REQUIRE
Outdated
@@ -1,3 +1,3 @@ | |||
julia 0.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undo?
@@ -137,12 +137,12 @@ function isrotation(r::AbstractMatrix{T}, tol::Real = 1000 * eps(eltype(T))) whe | |||
# Transpose is overloaded for many of our types, so we do it explicitly: | |||
r_trans = @SMatrix [conj(r[1,1]) conj(r[2,1]); | |||
conj(r[1,2]) conj(r[2,2])] | |||
d = vecnorm((r * r_trans) - eye(SMatrix{2,2})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just explicitly call Compat.norm
here instead of the conditional import in src/Rotations.jl
.
src/Rotations.jl
Outdated
@@ -8,14 +8,18 @@ using Compat.LinearAlgebra | |||
using StaticArrays | |||
|
|||
import Base: convert, eltype, size, length, getindex, *, Tuple | |||
import Compat.LinearAlgebra: inv, eye |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undo?
I think I fixed all of those now, not sure what I was thinking when I removed the Compat import. |
src/core_types.jl
Outdated
elseif size(r) == (3,3) | ||
r_trans = @SMatrix [conj(r[1,1]) conj(r[2,1]) conj(r[3,1]); | ||
conj(r[1,2]) conj(r[2,2]) conj(r[2,3]); | ||
conj(r[1,3]) conj(r[2,3]) conj(r[3,3])] | ||
d = vecnorm((r * r_trans) - eye(SMatrix{3,3})) | ||
d = norm((r * r_trans) - eye(SMatrix{3,3})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compat.norm
here as well.
Done |
Thank you! |
Ah oops, needs a new Compat version tag that includes JuliaLang/Compat.jl#577, my fault. |
It looks like 0.69 is the latest Compat version? |
Yeah, we'll have to wait for a new version to come out. No worries, Compat versions get tagged pretty frequently, we can wait for that before a new Rotations release. |
Very small PR for v0.7 update; the only code change is substituting
norm
forvecnorm
(deprecated). I'm not sure if my changes for CI and REQUIRE are correct as I don't have experience with package development myself.